Skip to content

Continue receive flow on offer/invoice creation failure#548

Merged
tnull merged 1 commit intolightningdevkit:mainfrom
Camillarhi:qr-receive-graceful-fallback
May 28, 2025
Merged

Continue receive flow on offer/invoice creation failure#548
tnull merged 1 commit intolightningdevkit:mainfrom
Camillarhi:qr-receive-graceful-fallback

Conversation

@Camillarhi
Copy link
Contributor

@Camillarhi Camillarhi commented May 26, 2025

Continue receive flow when BOLT12 offer or BOLT11 invoice creation fails, logging errors but
allowing onchain BIP21 as fallback.

Fixes #476

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 26, 2025 •
edited
Loading

👋 Thanks for assigning @tnull as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz May 26, 2025 23:21
@tnull tnull requested review from tnull and removed request for jkczyz May 27, 2025 10:33
tnull
tnull reviewed May 27, 2025
View reviewed changes
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but please also change the docs to match the new behavior. Also, can we add test coverage for this?

@Camillarhi
Copy link
Contributor Author

Camillarhi commented May 27, 2025

Looks good, but please also change the docs to match the new behavior. Also, can we add test coverage for this?

Thanks, I will make this update

@Camillarhi Camillarhi force-pushed the qr-receive-graceful-fallback branch from ddda6e5 to 72ee31a Compare May 27, 2025 19:09
@Camillarhi
Copy link
Contributor Author

Camillarhi commented May 27, 2025

Looks good, but please also change the docs to match the new behavior. Also, can we add test coverage for this?

Thanks, I will make this update

Hello @tnull, the docs and test coverage have been updated

enigbe
enigbe approved these changes May 28, 2025
View reviewed changes
Copy link
Contributor

@enigbe enigbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this and the changes LGTM.

tnull
tnull reviewed May 28, 2025
View reviewed changes
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few nits basically.

You'll also need to run cargo fmt to make the CI happy.

@Camillarhi Camillarhi force-pushed the qr-receive-graceful-fallback branch 2 times, most recently from 652ad4a to 03e065e Compare May 28, 2025 10:23
tnull
tnull reviewed May 28, 2025
View reviewed changes
@Camillarhi Camillarhi force-pushed the qr-receive-graceful-fallback branch from 03e065e to e396a1a Compare May 28, 2025 10:56
tnull
tnull approved these changes May 28, 2025
View reviewed changes
Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnull tnull merged commit 7977b04 into lightningdevkit:main May 28, 2025
9 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tnull tnull tnull approved these changes

+1 more reviewer

@enigbe enigbe enigbe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

unified_qr: Consider continuing receive flow on offer creation failure

4 participants

@Camillarhi @ldk-reviews-bot @tnull @enigbe